Skip to content

codemod iterations 5#2398

Merged
KKonstantinov merged 9 commits into
mainfrom
feature/codemod-iterations-5
Jul 1, 2026
Merged

codemod iterations 5#2398
KKonstantinov merged 9 commits into
mainfrom
feature/codemod-iterations-5

Conversation

@KKonstantinov

@KKonstantinov KKonstantinov commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Teach the v1→v2 codemod to catch two classes of test-code breakage that the handler migration leaves behind, and fix the batch-test harness so it can exercise monorepo targets that are their own pnpm workspace. Both codemod additions are advisory diagnostics — they flag and explain, they never rewrite.

Motivation and Context

The codemod already migrates handler implementations to v2 — reshaping extra.X reads to the nested ctx.mcpReq.* / ctx.http?.* context, and switching handler registration to the method-string key. But a migration is only load-bearing if the project's tests still pass afterward, and two batch tests surfaced cases where the sources migrate cleanly yet the tests break at runtime because the codemod can't safely reach the test doubles:

  • v1 mock contexts (mcp-servers batch test). A test hands a hand-built context to a bare handler(args, mockCtx) call. The codemod reshapes reads inside handler definitions it can anchor on (registerTool, setRequestHandler, fallback handlers, annotated params), but a free-standing object literal passed to an invocation is never reached, so it keeps the flat v1 shape. The migrated handler then reads ctx.mcpReq.send / .id against it and throws Cannot read properties of undefined (reading 'send'). In the mcp-servers fork this was 11 failures from 1 root cause.
  • Stale registration-schema assertions. In v2 handlers register by the method string, not by the *RequestSchema / *NotificationSchema constant. Tests that assert against a setRequestHandler mock's recorded first argument (expect(schemas).toContain(SubscribeRequestSchema)) or look a handler up by schema (calls.find(c => c[0] === SubscribeRequestSchema)) silently go stale — the same constant is still importable and still valid for .parse(), so nothing errors at compile time; the assertion just never matches.

Neither can be auto-rewritten safely: an untyped object literal that merely shares a key name might not be a context mock, and a schema constant used in a registration-shaped expression might be a legitimate .parse()/validation use. So the codemod's job here is to diagnose precisely — point at the node, name the v2 shape, and let the author apply it.

Separately, the batch-test harness couldn't run monorepo targets that carry their own pnpm-workspace.yaml (catalog:/workspace: deps — e.g. mastra). The install path unconditionally passed --ignore-workspace, which discards the clone's workspace file; pnpm then can't resolve the catalog/workspace links (ERR_PNPM_CATALOG_ENTRY_NOT_FOUND_FOR_SPEC) and the repo is skipped — so those targets never produced codemod signal.

What this PR adds:

  • contextTypes — v1 mock-context detection. Flags object literals in a call-argument or variable-initializer position that look like a v1 handler-context mock: either one distinctive v1 key (sendRequest, sendNotification, requestId, requestInfo, authInfo, closeSSEStream, closeStandaloneSSEStream) or ≥2 context-like keys together. Emits an advisory diagnostic with a concrete reshape hint per key (sendRequest → mcpReq.send, requestId → mcpReq.id, …), a worked example ({ sendRequest: fn } → { mcpReq: { send: fn } }), and a note that sessionId stays top-level. Skips literals already in v2 shape (containing mcpReq / http / task). Never rewrites.
  • handlerRegistration — stale registration-schema flagging. In a file that performs handler registration, flags a method-mapped schema constant used as a value (a call argument, or an ===/!==/==/!= operand) and advises comparing against the method string instead ('resources/subscribe'). Only fires in files that still reference the schema — a fully converted source drops the import, so this targets the registration tests that survive. Never rewrites.
  • Batch-test harness — own-workspace clones. installCommand now detects a clone with its own pnpm-workspace.yaml and keeps it, scoping the install to the target packages via --filter "./<dir>..." (installing only what the checks need — e.g. 2 of mastra's 161 projects — instead of the whole tree). The single-package clone path (--ignore-workspace, so pnpm doesn't walk up into the SDK's own workspace) is unchanged. The command is computed once and reused for the post-codemod reinstall.
  • Generated version pin bump. V2_PACKAGE_VERSIONS regenerated from 2.0.0-alpha.32.0.0-beta.1, so the codemod's package.json dependency rewrite now targets the beta line.

How Has This Been Tested?

  • Unit tests, contextTypes (7 new cases): inline call-argument mock, variable-bound mock with the sessionId-stays-top-level note, the _meta/requestId progress-notification shape, two-generic-keys-without-a-distinctive-key, and three negatives — already-v2 shape, a non-context config literal, and a lone generic key. Each positive asserts advisory-only and the exact reshape hint; the literal is asserted left untouched.
  • Unit tests, handlerRegistration (4 new cases): schema as a toContain assertion arg, schema as a registration-lookup helper arg, and two negatives — schema used as .parse() alongside a real registration, and a schema referenced in a file with no handler registration.
  • Unit tests, installCommand (4 new cases): non-pnpm managers, pnpm single-package clone (--ignore-workspace), pnpm own-workspace clone (no --ignore-workspace, --filter per target package), and own-workspace clone whose only target is the root (whole-workspace install, no --filter).
  • Full codemod suite green: 19 files, 615 tests passing.
  • The diagnostics were derived from real batch-test runs (mcp-servers fork for the mock-context gap; the memory/subscribe server for the registration-schema gap); the harness fix was validated by getting mastra (own pnpm workspace) to install and run its checks instead of being skipped.

Breaking Changes

None to any public API. The change is additive to the codemod and the batch-test harness. The two new codemod outputs are advisory diagnostics only — no existing migration output changes; the codemod now surfaces more actionable guidance about test code it was previously silent on.

Types of changes

  • Bug fix (non-breaking change which fixes an issue) — codemod was silent on v1 mock contexts and stale registration-schema assertions that break migrated tests at runtime
  • New feature (non-breaking change which adds functionality) — two new advisory diagnostics + batch-test support for own-workspace monorepo targets
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

  • Advisory, not rewritten — by design. Both new detections sit on top of ambiguity the AST can't resolve: an untyped literal sharing a key name might not be a context mock, and a schema constant is still a valid .parse() argument. Rewriting either risks corrupting correct code, so the codemod flags with a precise, node-anchored message instead. This mirrors the existing advisoryOnly diagnostics for fallback handlers and annotated params.
  • Detection precision. The mock-context check requires either a distinctive v1 key or ≥2 context-like keys, and bails on anything already in the v2 nested shape, to avoid firing on ordinary config/option objects. The registration-schema check only runs in files that actually call setRequestHandler/setNotificationHandler and only flags value positions (call arg / equality operand), leaving property-access bases like Schema.parse(x) alone.
  • Batch-test scoping. For own-workspace targets the install is filtered to ./<dir>... (package + its dependencies) so a large monorepo installs only the projects under test rather than the entire workspace — the difference between a runnable target and a multi-minute/timeout install.
  • Note for reviewers following this branch: an earlier iteration explored a CommonJS-interop transform (dynamic-import() rewriting for CJS targets). That is not part of this change — feat(packaging): ship CommonJS builds alongside ESM for v2 packages #2405 shipped native .cjs builds for the v2 packages, so CJS projects consume v2 directly and the interop transform was dropped. The net diff here is the two diagnostics, the harness fix, and the version-pin bump.

@KKonstantinov KKonstantinov requested a review from a team as a code owner June 30, 2026 19:33
@changeset-bot

changeset-bot Bot commented Jun 30, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: b6010cf

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Jun 30, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2398

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2398

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2398

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2398

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2398

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2398

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2398

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2398

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2398

commit: b6010cf

Comment thread packages/codemod/src/migrations/v1-to-v2/transforms/commonjsInterop.ts Outdated

@felixweinberger felixweinberger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting back in your queue for fixing the build issue + bot finding

@KKonstantinov

Copy link
Copy Markdown
Contributor Author

@claude review

@KKonstantinov

Copy link
Copy Markdown
Contributor Author

@claude review

claude[bot]

This comment was marked as outdated.

@KKonstantinov

Copy link
Copy Markdown
Contributor Author

@claude review

Comment thread packages/codemod/src/bin/batchTest.ts
Comment thread packages/codemod/src/migrations/v1-to-v2/transforms/contextTypes.ts
Comment thread packages/codemod/src/migrations/v1-to-v2/transforms/contextTypes.ts
Comment thread packages/codemod/src/migrations/v1-to-v2/transforms/contextTypes.ts
Comment thread packages/codemod/src/migrations/v1-to-v2/transforms/handlerRegistration.ts Outdated
@KKonstantinov KKonstantinov merged commit 1772473 into main Jul 1, 2026
20 checks passed
@KKonstantinov KKonstantinov deleted the feature/codemod-iterations-5 branch July 1, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants